London | 26-ATP-Jan | Boualem Larbi Djebbour | Sprint 2 | coursework#1188
London | 26-ATP-Jan | Boualem Larbi Djebbour | Sprint 2 | coursework#1188djebsoft wants to merge 15 commits intoCodeYourFuture:mainfrom
Conversation
…perty, and false otherwise
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| // Given invalid parameters like an array | ||
| // When passed to contains | ||
| // Then it should return false or throw an error | ||
| test("given invalid parameters, returns false or throws an error", () => { | ||
| expect(contains(["gitName", "position"], "gitName")).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
This test does not yet confirm that the function correctly returns false when the first argument is an array.
This is because contains(["gitName", "position"], "gitName") could also return false simply because "gitName" is not a key of the array.
Arrays are objects, with their indices acting as keys. A proper test should use a valid
key to ensure the function returns false specifically because the input is an array, not because the key is missing.
After you fixed this test, make sure you also run the test to check your function.
Note: When testing invalid type of data, undefined and null are usually good candidates to test -- many functions fail because they could not handle undefined or null.
There was a problem hiding this comment.
thank you for the feedback
I just realised that the false returned cases should be the exception, so I changed the if condition to the other way around.
I removed the case of empty object (object length equals to zero) as it does not add any plus to the code.
I also added the case where the input is an array to return false.
and run the test to check the function.
added a test with null input for invalid type of data.
There was a problem hiding this comment.
Why not also update the test (the one that tests array)?
Your function is correct, but we write tests not only to verify our current implementation, but also to ensure that future changes do not alter the function's expected behavior.
There was a problem hiding this comment.
thanks for the spot
sorry that I got confused about this point
i think the perfect example test is to test tricky arguments where the first argument looks like an object by having two elements and the second argument is string that match one the elements in the first argument
therefore, if the first argument wasn't an array than the test should return true. but because it an array, the function returns false as invalid input.
could you enlighten me about what I'm mistaken here ?
There was a problem hiding this comment.
expect(contains(["gitName", "position"], "gitName")).toEqual(false);
The following function (changed in the future by someone else) could also pass the above test:
function contains(obj, key) {
if (obj == null || typeof obj != "object") return false;
return Object.hasOwn(obj, key);
}
There was a problem hiding this comment.
This test is supposed to fail when the function is implemented as:
function contains(obj, key) {
if (obj == null || typeof obj != "object") return false;
return Object.hasOwn(obj, key);
}The new test still behaves the same as the previous one.
Arrays are objects, with their indices acting as keys. You should use a valid
key to ensure the function returns false specifically because the input is an array, not because the key is missing.
Learners, PR Template
Self checklist
Changelist
completed sprint-2 exercies in data groups module.
Questions